refactor: Move enter action into its own class. #306
Conversation
cpcallen
left a comment
cpcallen
left a comment
There was a problem hiding this comment.
I am about to add another meaning for the enter key: when moving a block, pressing enter will place the block at its current location. I was going to just wire it in to handleEnterForWS but as this PR is now in-flight it occured to me that a better approach is to do what we should eventually do anyway, and make it a completely separate action. Similarly, I think the several unrelated activities here should be split in to separate actions:
- One which, when on a flyout button, presses the button.
- One which, when on a flyout stack, creates new block(s) on the workspace.
- One which, when on a field, opens the field editor.
- One which, when on a connection or the workspace, runs the insert action (this should be in
insert.ts) - One which, when none of the above apply, shows the help message about the context menu.
- This needs to be registered first, since shortcuts are tried in the reverse of the order they are registered in.
It might be reasonable for 1 and 2 to be kept as part of a single action, but the other three are unrelated and ought to be separated out. (No objection to doing this as part of a separate follow-up PR, or even just filing a bug to do it later.)
|
Let's still merge this PR, and land your new meaning of Enter, and then assess. I am leery of splitting this into multiple actions because it spreads out the logic for deciding which action to do/which takes prioritiy. |
This PR fixes #297 by moving the Enter action into its own class, and moving several helper functions used only by that action into its class and out of navigation.ts. This also incorporates changes from #273 in
insertFromFlyout(), which was moved into the enter action class.